Migrating from setuptools to uv #114

chinacat567 · 2025-05-12T14:15:53Z

Context

Read discussion here

Description

This PR is moving away from setuptools to uv to manage dependencies
A publish workflow can then be used to build and publish dicomweb-client to PyPI (PR to follow soon)

.github/workflows/publish-client.yml

medihack · 2025-05-13T15:51:35Z

@chinacat567 In this case, it would make sense to call the workflow only publish.yml (and not publish-client.yml).

@CPBridge To automatically publish to PyPI when publishing on GitHub, you must allow this on the PyPI homepage by associating the GitHub repo and workflow with the package (it's quite simple).

It would look like this in your case:

PyPI Project Name (required)
dicomweb-client

Owner (required)
ImagingDataCommons

Repository name (required)
dicomweb-client

Workflow name (required)
publish.yml

Environment name (optional) 
(I left this empty)

medihack · 2025-05-14T08:17:14Z

@chinacat567 I also think that the configurations in setup.cfg can be moved (most probably need to be adapted) into pyproject.toml. I also don't see a need for the setup.py then anymore.

chinacat567 · 2025-05-14T10:03:23Z

@chinacat567 I also think that the configurations in setup.cfg can be moved (most probably need to be adapted) into pyproject.toml. I also don't see a need for the setup.py then anymore.

Deleted setup.py and setup.cfg, moving flake8 settings to a .flake8 file.

medihack · 2025-05-14T10:55:57Z

LGTM now. Looking forward to an upcoming release 🙂.

CPBridge · 2025-05-14T18:41:16Z

Thanks! Bit underwater for the next couple of days but I'll take a look later in the week

CPBridge

Thanks for this nice upgrade @chinacat567 @medihack. I have a few specific comments on the code, and also a question.

I've personally not used automatic versioning before. Can you confirm what is used as the source of the version? Is it the release title or the git tag? I ask because we prepend av in the git tags, but not in the release titles or the version strings and I want to make sure we don't mess this up...

Secondly, there is currently a version number stored within the source code itself in the top level __init__.py such that someone can access dicomweb_client.__version__ within python. From what I can tell, the automatic versioning introduced by this PR would not affect that version string, which would seem to be a big problem. I see the poetry dynamic versioning tool has the capability to do this, but the uv version is very poorly documented as far as I can tell. From what you know, is it possible to have this version string updated automatically too?

pyproject.toml

.flake8

.github/workflows/publish.yml

pyproject.toml

CPBridge · 2025-05-17T12:43:14Z

uv.lock

@@ -0,0 +1,1226 @@
+version = 1


From my understanding, whether to commit a lockfile for a a library project such as this is a matter of debate. It is not required for anyone installing the library (which simply uses the dependency ranges listed in pyproject.toml) and is basically just for developers to have synced environments. But I imagine this will create a lot of noise in commits as these environments are updated.

I generally come down on the side of not committing lock files, but I could be persuaded if there are specific benefits we get from it (personally I am much less familiar with uv than poetry, which I have mostly been using for the last few years).

Thoughts?

You are right that the usage of lock files is somehow debatable, but I guess, with the increasing spread of uv (and poetry), those are becoming more the default than the exception. I like them for the reproducibility and consistency of my development and testing environment. I also use renovate to automatically update all my dependencies (including the lock files).

I deleted the uv.lock file and put *.lock into .gitignore. So it's gone.

Ok thanks, we can maybe revisit if there's a reason in the future. I should clarify that I'm talking purely about library repos, for other types of project (applications) I think it makes a lot of sense.

That said it seems that you added the lock file to the gitignore but didn't actually git rm it, it's still there! @medihack

medihack · 2025-05-18T11:09:34Z

I've personally not used automatic versioning before. Can you confirm what is used as the source of the version? Is it the release title or the git tag? I ask because we prepend av in the git tags, but not in the release titles or the version strings and I want to make sure we don't mess this up...

Ah, ok. I wasn't aware that your tags use a "v" prefix. Dynamic versioning indeed uses the tag and not the release title.
@chinacat567 No need for pattern = "default-unprefixed"

Secondly, there is currently a version number stored within the source code itself in the top level __init__.py such that someone can access dicomweb_client.__version__ within python. From what I can tell, the automatic versioning introduced by this PR would not affect that version string, which would seem to be a big problem. I see the poetry dynamic versioning tool has the capability to do this, but the uv version is very poorly documented as far as I can tell. From what you know, is it possible to have this version string updated automatically too?

You are right. That is a shortcoming I also wasn't aware of. And unfortunately, I don't know an easy solution for it 😞. The uv team plans to integrate the dynamic versioning in uv itself. Hopefully, they will support that use case as well. But it is not there yet.

That said, maybe uv isn't a good fit here (or at least the dynamic versioning and publish workflow). It was just a suggestion to integrate a more "modern" build system (but modern is, of course, not always better, or at least not always a better fit). So if you are happy with the current build system (and it also seems to work well), I am also ok with just closing this PR (and having a release with the current build system 😉).

CPBridge · 2025-05-18T18:42:00Z

You are right. That is a shortcoming I also wasn't aware of. And unfortunately, I don't know an easy solution for it 😞. The uv team plans to integrate the dynamic versioning in uv itself. Hopefully, they will support that use case as well. But it is not there yet.

I think perhaps this (the hook) could be a solution?

medihack · 2025-05-19T11:41:07Z

I think perhaps this (the hook) could be a solution?

Ah interesting ... I didn't know about that. I used importlib.metadata (I guess performance isn't that critical).

medihack · 2025-05-19T11:46:36Z

I can't check if the correct git tag is picked up as a version. So, if you really want to merge it, try it out manually first before publishing a new release by running uv build.

CPBridge

Thanks!

So, if you really want to merge it, try it out manually first before publishing a new release by running uv build.

I just tried this and it seems to work correctly. I.e. building when at commit tagged with a v (e.g. v0.60.0) produces a version without a v (0.60.0) and when imported, the module's __version__ attribute is the tag without the v (0.60.0)

Just one little thing about the lock file...

CPBridge · 2025-05-19T16:22:40Z

uv.lock

@@ -0,0 +1,1226 @@
+version = 1


Ok thanks, we can maybe revisit if there's a reason in the future. I should clarify that I'm talking purely about library repos, for other types of project (applications) I think it makes a lot of sense.

That said it seems that you added the lock file to the gitignore but didn't actually git rm it, it's still there! @medihack

CPBridge · 2025-05-19T16:30:16Z

Also, any concerns from @pieper before finalising?

sonarqubecloud · 2025-05-20T08:29:37Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

medihack · 2025-05-20T08:34:24Z

uv.lock is removed (I just deleted it and added it to .gitignore in the same commit, which of course can't work 🙈).

Another good addition would be to switch from flake8 to ruff. But let's tackle that in a separate PR sometime later.

migrating from setuptools to uv

d0651b1

chinacat567 mentioned this pull request May 12, 2025

Additional get params #108

Merged

chinacat567 marked this pull request as draft May 12, 2025 16:59

adding publish workflow using uv and dynamic versioning

ea53b10

chinacat567 marked this pull request as ready for review May 13, 2025 14:20

medihack reviewed May 13, 2025

View reviewed changes

.github/workflows/publish-client.yml Outdated Show resolved Hide resolved

pypi url fix and workflow file rename

38ae99e

chinacat567 added 2 commits May 14, 2025 10:23

deleting old workflow file

7e49a87

deleting setuptools artifacts

198abb5

CPBridge self-assigned this May 14, 2025

CPBridge added the enhancement New feature or request label May 14, 2025

CPBridge requested changes May 17, 2025

View reviewed changes

medihack added 6 commits May 19, 2025 12:54

Fix formatting

90bdc07

Rename dev group back to test group in pyproject.toml

1cf691a

Permanently remove lock file from repo

4edcaf4

Install all dependency-groups by default

e3edfe7

Use default to fetch version from git tag (with v prefix)

0e9b6dd

Get __version__ attribute from importlib metadata

54c29ee

CPBridge requested changes May 19, 2025

View reviewed changes

Remove uv.lock from index

a915f3f

CPBridge approved these changes May 20, 2025

View reviewed changes

CPBridge merged commit ded157f into ImagingDataCommons:master May 24, 2025
7 checks passed

Migrating from setuptools to uv #114

Migrating from setuptools to uv #114

Uh oh!

Conversation

chinacat567 commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Description

Uh oh!

Uh oh!

medihack commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

medihack commented May 14, 2025

Uh oh!

chinacat567 commented May 14, 2025

Uh oh!

medihack commented May 14, 2025

Uh oh!

CPBridge commented May 14, 2025

Uh oh!

CPBridge left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CPBridge May 17, 2025

Choose a reason for hiding this comment

Uh oh!

medihack May 18, 2025

Choose a reason for hiding this comment

Uh oh!

medihack May 19, 2025

Choose a reason for hiding this comment

Uh oh!

CPBridge May 19, 2025

Choose a reason for hiding this comment

Uh oh!

medihack commented May 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CPBridge commented May 18, 2025

Uh oh!

medihack commented May 19, 2025

Uh oh!

medihack commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CPBridge left a comment

Choose a reason for hiding this comment

Uh oh!

CPBridge May 19, 2025

Choose a reason for hiding this comment

Uh oh!

CPBridge commented May 19, 2025

Uh oh!

sonarqubecloud bot commented May 20, 2025

Quality Gate passed

Uh oh!

medihack commented May 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chinacat567 commented May 12, 2025 •

edited

Loading

medihack commented May 13, 2025 •

edited

Loading

medihack commented May 18, 2025 •

edited

Loading

medihack commented May 19, 2025 •

edited

Loading